Skip to content

[flang][acc] Create UseDeviceOp for both results of hlfir.declare #148017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[flang][acc] Create UseDeviceOp for both results of hlfir.declare #148017

wants to merge 6 commits into from
+100 −16

Conversation

nvptm
Copy link
Contributor

@nvptm nvptm commented Jul 10, 2025

A sample such as

program test
  integer :: N = 100
  real*8 :: b(-1:N)
  !$acc data copy(b)
  !$acc host_data use_device(b)
  call vadd(b)
  !$acc end host_data
  !$acc end data
end

is lowered to

    %13:2 = hlfir.declare %11(%12) {uniq_name = "_QFEb"} : (!fir.ref<!fir.array<?xf64>>, !fir.shapeshift<1>) -> (!fir.box<!fir.array<?xf64>>, !fir.ref<!fir.array<?xf64>>)
    %14 = acc.copyin var(%13#0 : !fir.box<!fir.array<?xf64>>) -> !fir.box<!fir.array<?xf64>> {dataClause = #acc<data_clause acc_copy>, name = "b"}
    acc.data dataOperands(%14 : !fir.box<!fir.array<?xf64>>) {
      %15 = acc.use_device var(%13#0 : !fir.box<!fir.array<?xf64>>) -> !fir.box<!fir.array<?xf64>> {name = "b"}
      acc.host_data dataOperands(%15 : !fir.box<!fir.array<?xf64>>) {
        fir.call @_QPvadd(%13#1) fastmath<contract> : (!fir.ref<!fir.array<?xf64>>) -> ()
        acc.terminator
      }
      acc.terminator
    }
    acc.copyout accVar(%14 : !fir.box<!fir.array<?xf64>>) to var(%13#0 : !fir.box<!fir.array<?xf64>>) {dataClause = #acc<data_clause acc_copy>, name = "b"}

Note that while the use_device clause is applied to %13#0, the argument passed to vadd is %13#1. To avoid problems later in lowering, this change additionally applies the use_device clause to %13#1, so that the resulting MLIR is

   %13:2 = hlfir.declare %11(%12) {uniq_name = "_QFEb"} : (!fir.ref<!fir.array<?xf64>>, !fir.shapeshift<1>) -> (!fir.box<!fir.array<?xf64>>, !fir.ref<!fir.array<?xf64>>)
    %14 = acc.copyin var(%13#0 : !fir.box<!fir.array<?xf64>>) -> !fir.box<!fir.array<?xf64>> {dataClause = #acc<data_clause acc_copy>, name = "b"}
    acc.data dataOperands(%14 : !fir.box<!fir.array<?xf64>>) {
      %15 = acc.use_device var(%13#0 : !fir.box<!fir.array<?xf64>>) -> !fir.box<!fir.array<?xf64>> {name = "b"}
      %16 = acc.use_device varPtr(%13#1 : !fir.ref<!fir.array<?xf64>>) -> !fir.ref<!fir.array<?xf64>> {name = "b"}
      acc.host_data dataOperands(%15, %16 : !fir.box<!fir.array<?xf64>>, !fir.ref<!fir.array<?xf64>>) {
        fir.call @_QPvadd(%13#1) fastmath<contract> : (!fir.ref<!fir.array<?xf64>>) -> ()
        acc.terminator
      }
      acc.terminator
    }
    acc.copyout accVar(%14 : !fir.box<!fir.array<?xf64>>) to var(%13#0 : !fir.box<!fir.array<?xf64>>) {dataClause = #acc<data_clause acc_copy>, name = "b"}

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir openacc labels Jul 10, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2025 •
edited
Loading

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-openacc

Author: None (nvptm)

Changes

A sample such as

program test
  integer :: N = 100
  real*8 :: b(-1:N)
  !$acc data copy(b)
  !$acc host_data use_device(b)
  call vadd(b)
  !$acc end host_data
  !$acc end data
end

is lowered to

    %13:2 = hlfir.declare %11(%12) {uniq_name = "_QFEb"} : (!fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;, !fir.shapeshift&lt;1&gt;) -&gt; (!fir.box&lt;!fir.array&lt;?xf64&gt;&gt;, !fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;)
    %14 = acc.copyin var(%13#<!-- -->0 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) -&gt; !fir.box&lt;!fir.array&lt;?xf64&gt;&gt; {dataClause = #acc&lt;data_clause acc_copy&gt;, name = "b"}
    acc.data dataOperands(%14 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) {
      %15 = acc.use_device var(%13#<!-- -->0 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) -&gt; !fir.box&lt;!fir.array&lt;?xf64&gt;&gt; {name = "b"}
      acc.host_data dataOperands(%15 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) {
        fir.call @<!-- -->_QPvadd(%13#<!-- -->1) fastmath&lt;contract&gt; : (!fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;) -&gt; ()
        acc.terminator
      }
      acc.terminator
    }
    acc.copyout accVar(%14 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) to var(%13#<!-- -->0 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) {dataClause = #acc&lt;data_clause acc_copy&gt;, name = "b"}

Note that while the use_device clause is applied to %13#0, the argument passed to vadd is %13#1. To avoid problems later in lowering, this change additionally applies the use_device clause to %13#1, so that the resulting MLIR is

   %13:2 = hlfir.declare %11(%12) {uniq_name = "_QFEb"} : (!fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;, !fir.shapeshift&lt;1&gt;) -&gt; (!fir.box&lt;!fir.array&lt;?xf64&gt;&gt;, !fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;)
    %14 = acc.copyin var(%13#<!-- -->0 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) -&gt; !fir.box&lt;!fir.array&lt;?xf64&gt;&gt; {dataClause = #acc&lt;data_clause acc_copy&gt;, name = "b"}
    acc.data dataOperands(%14 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) {
      %15 = acc.use_device var(%13#<!-- -->0 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) -&gt; !fir.box&lt;!fir.array&lt;?xf64&gt;&gt; {name = "b"}
      %16 = acc.use_device varPtr(%13#<!-- -->1 : !fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;) -&gt; !fir.ref&lt;!fir.array&lt;?xf64&gt;&gt; {name = "b"}
      acc.host_data dataOperands(%15, %16 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;, !fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;) {
        fir.call @<!-- -->_QPvadd(%13#<!-- -->1) fastmath&lt;contract&gt; : (!fir.ref&lt;!fir.array&lt;?xf64&gt;&gt;) -&gt; ()
        acc.terminator
      }
      acc.terminator
    }
    acc.copyout accVar(%14 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) to var(%13#<!-- -->0 : !fir.box&lt;!fir.array&lt;?xf64&gt;&gt;) {dataClause = #acc&lt;data_clause acc_copy&gt;, name = "b"}

Full diff: https://github.com/llvm/llvm-project/pull/148017.diff

3 Files Affected:

  • (modified) flang/lib/Lower/OpenACC.cpp (+13)
  • (modified) flang/test/Lower/OpenACC/acc-host-data-unwrap-defaultbounds.f90 (+8-6)
  • (modified) flang/test/Lower/OpenACC/acc-host-data.f90 (+12-9)
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index 42842bcb41a74..23481d0ef7935 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -738,6 +738,19 @@ genDataOperandOperations(const Fortran::parser::AccObjectList &objectList,
         implicit, dataClause, baseAddr.getType(), async, asyncDeviceTypes,
         asyncOnlyDeviceTypes, /*unwrapBoxAddr=*/true, info.isPresent);
     dataOperands.push_back(op.getAccVar());
+    // For UseDeviceOp, if operand is one of a pair resulting from a
+    // declare operation, create a UseDeviceOp for the other operand as well.
+    if constexpr (std::is_same_v<Op, mlir::acc::UseDeviceOp>) {
+      if (mlir::isa<hlfir::DeclareOp>(baseAddr.getDefiningOp())) {
+        Op op = createDataEntryOp<Op>(
+            builder, operandLocation, baseAddr.getDefiningOp()->getResult(1),
+            asFortran, bounds, structured, implicit, dataClause,
+            baseAddr.getDefiningOp()->getResult(1).getType(), async,
+            asyncDeviceTypes, asyncOnlyDeviceTypes, /*unwrapBoxAddr=*/true,
+            info.isPresent);
+        dataOperands.push_back(op.getAccVar());
+      }
+    }
   }
 }
 
diff --git a/flang/test/Lower/OpenACC/acc-host-data-unwrap-defaultbounds.f90 b/flang/test/Lower/OpenACC/acc-host-data-unwrap-defaultbounds.f90
index 164eb32a8f684..2de7cc5761a2b 100644
--- a/flang/test/Lower/OpenACC/acc-host-data-unwrap-defaultbounds.f90
+++ b/flang/test/Lower/OpenACC/acc-host-data-unwrap-defaultbounds.f90
@@ -15,15 +15,17 @@ subroutine acc_host_data()
   !$acc end host_data
 
 ! CHECK: %[[BOUND:.*]] = acc.bounds lowerbound(%{{.*}} : index) upperbound(%{{.*}} : index) stride(%{{.*}} : index) startIdx(%{{.*}} : index)
-! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
-! CHECK: acc.host_data dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>)
+! CHECK: %[[DA0:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: %[[DA1:.*]] = acc.use_device varPtr(%[[DECLA]]#1 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+ ! CHECK: acc.host_data dataOperands(%[[DA0]], %[[DA1]] : !fir.ref<!fir.array<10xf32>>, !fir.ref<!fir.array<10xf32>>)
 
   !$acc host_data use_device(a) if_present
   !$acc end host_data
 
 ! CHECK: %[[BOUND:.*]] = acc.bounds lowerbound(%{{.*}} : index) upperbound(%{{.*}} : index) stride(%{{.*}} : index) startIdx(%{{.*}} : index)
-! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
-! CHECK: acc.host_data dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>) {
+! CHECK: %[[DA0:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: %[[DA1:.*]] = acc.use_device varPtr(%[[DECLA]]#1 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: acc.host_data dataOperands(%[[DA0]], %[[DA1]] : !fir.ref<!fir.array<10xf32>>{{.*}}) {
 ! CHECK: } attributes {ifPresent}
 
   !$acc host_data use_device(a) if(ifCondition)
@@ -33,14 +35,14 @@ subroutine acc_host_data()
 ! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
 ! CHECK: %[[LOAD_IFCOND:.*]] = fir.load %[[DECLIFCOND]]#0 : !fir.ref<!fir.logical<4>>
 ! CHECK: %[[IFCOND_I1:.*]] = fir.convert %[[LOAD_IFCOND]] : (!fir.logical<4>) -> i1
-! CHECK: acc.host_data if(%[[IFCOND_I1]]) dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>)
+! CHECK: acc.host_data if(%[[IFCOND_I1]]) dataOperands(%[[DA]]{{.*}} : !fir.ref<!fir.array<10xf32>>{{.*}})
 
   !$acc host_data use_device(a) if(.true.)
   !$acc end host_data
 
 ! CHECK: %[[BOUND:.*]] = acc.bounds lowerbound(%{{.*}} : index) upperbound(%{{.*}} : index) stride(%{{.*}} : index) startIdx(%{{.*}} : index)
 ! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) bounds(%[[BOUND]]) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
-! CHECK: acc.host_data dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>)
+! CHECK: acc.host_data dataOperands(%[[DA]]{{.*}} : !fir.ref<!fir.array<10xf32>>{{.*}})
 
   !$acc host_data use_device(a) if(.false.)
     a = 1.0
diff --git a/flang/test/Lower/OpenACC/acc-host-data.f90 b/flang/test/Lower/OpenACC/acc-host-data.f90
index 871eabd256ca6..4d09b25b983b9 100644
--- a/flang/test/Lower/OpenACC/acc-host-data.f90
+++ b/flang/test/Lower/OpenACC/acc-host-data.f90
@@ -14,34 +14,37 @@ subroutine acc_host_data()
   !$acc host_data use_device(a)
   !$acc end host_data
 
-! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
-! CHECK: acc.host_data dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>)
+! CHECK: %[[DA0:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: %[[DA1:.*]] = acc.use_device varPtr(%[[DECLA]]#1 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: acc.host_data dataOperands(%[[DA0]], %[[DA1]] : !fir.ref<!fir.array<10xf32>>, !fir.ref<!fir.array<10xf32>>)
 
   !$acc host_data use_device(a) if_present
   !$acc end host_data
 
-! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
-! CHECK: acc.host_data dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>) {
+! CHECK: %[[DA0:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: %[[DA1:.*]] = acc.use_device varPtr(%[[DECLA]]#1 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: acc.host_data dataOperands(%[[DA0]], %[[DA1]] : !fir.ref<!fir.array<10xf32>>, !fir.ref<!fir.array<10xf32>>)
 ! CHECK: } attributes {ifPresent}
 
-  !$acc host_data use_device(a) if_present if_present
+  !$acc host_data use_device(a) if_present 
   !$acc end host_data
-! CHECK: acc.host_data dataOperands(%{{.*}} : !fir.ref<!fir.array<10xf32>>) {
+! CHECK: acc.host_data dataOperands(%{{.*}}{{.*}} : !fir.ref<!fir.array<10xf32>>{{.*}}) {
 ! CHECK: } attributes {ifPresent}
 
   !$acc host_data use_device(a) if(ifCondition)
   !$acc end host_data
 
-! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: %[[DA0:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
+! CHECK: %[[DA1:.*]] = acc.use_device varPtr(%[[DECLA]]#1 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
 ! CHECK: %[[LOAD_IFCOND:.*]] = fir.load %[[DECLIFCOND]]#0 : !fir.ref<!fir.logical<4>>
 ! CHECK: %[[IFCOND_I1:.*]] = fir.convert %[[LOAD_IFCOND]] : (!fir.logical<4>) -> i1
-! CHECK: acc.host_data if(%[[IFCOND_I1]]) dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>)
+! CHECK: acc.host_data if(%[[IFCOND_I1]]) dataOperands(%[[DA0]]{{.*}} : !fir.ref<!fir.array<10xf32>>{{.*}})
 
   !$acc host_data use_device(a) if(.true.)
   !$acc end host_data
 
 ! CHECK: %[[DA:.*]] = acc.use_device varPtr(%[[DECLA]]#0 : !fir.ref<!fir.array<10xf32>>) -> !fir.ref<!fir.array<10xf32>> {name = "a"}
-! CHECK: acc.host_data dataOperands(%[[DA]] : !fir.ref<!fir.array<10xf32>>)
+! CHECK: acc.host_data dataOperands(%[[DA]]{{.*}} : !fir.ref<!fir.array<10xf32>>{{.*}})
 
   !$acc host_data use_device(a) if(.false.)
     a = 1.0

@razvanlupusoru razvanlupusoru self-requested a review July 10, 2025 18:23
@razvanlupusoru razvanlupusoru changed the title [mlir][acc] Create UseDeviceOp for both results of hlfir.declare [flang][acc] Create UseDeviceOp for both results of hlfir.declare Jul 10, 2025
razvanlupusoru
razvanlupusoru reviewed Jul 10, 2025
View reviewed changes
Copy link
Contributor

@razvanlupusoru razvanlupusoru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add test which matches the issue you ran into and the commit message?

Also, can you also check and add test for Fortran assumed-shape, pointer, and allocatable passed to a function (bindc or assumed-size) that also uses the same pattern? I want to make sure that we handle those cases correctly also.

flang/lib/Lower/OpenACC.cpp Outdated
if constexpr (std::is_same_v<Op, mlir::acc::UseDeviceOp>) {
if (mlir::isa<hlfir::DeclareOp>(baseAddr.getDefiningOp())) {
Op op = createDataEntryOp<Op>(
builder, operandLocation, baseAddr.getDefiningOp()->getResult(1),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please assert or check that baseAddr and getResult(1) are not the same?

flang/lib/Lower/OpenACC.cpp Outdated
builder, operandLocation, baseAddr.getDefiningOp()->getResult(1),
asFortran, bounds, structured, implicit, dataClause,
baseAddr.getDefiningOp()->getResult(1).getType(), async,
asyncDeviceTypes, asyncOnlyDeviceTypes, /*unwrapBoxAddr=*/true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please create a "const bool unwrapBoxAddr = true;*/ and use unwrapBoxAddr in both spots (to ensure they do not go out of sync).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@razvanlupusoru razvanlupusoru razvanlupusoru left review comments

Assignees
No one assigned
Labels
flang:fir-hlfir flang Flang issues not falling into any other category openacc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nvptm @llvmbot @razvanlupusoru